Skip to content

Add NEON tests the test/entropy program. - #160

Open
jkbonfield wants to merge 1 commit into
samtools:masterfrom
jkbonfield:neon-tests
Open

jkbonfield wants to merge 1 commit into
samtools:masterfrom
jkbonfield:neon-tests

Conversation

@jkbonfield

@jkbonfield jkbonfield commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Neon is -c 0x0808, but we only tested from 0000 to 0404. (On x86 this doesn't matter - it'll just be a scalar test, just as the AVX ones on NEON are too.)

Also use ulimit -s instead of setrlimit as it appears the Mac may ignore some aspects of this, invalidating the stack size checks.

A companion to #159 which this change is designed to validate the error prior to fixing it, and to ensure we're checking everything we believed we were.

@daviesrob daviesrob self-assigned this Nov 12, 2026
Comment thread tests/entropy.test Outdated
# Four copies tests the large buffer histogram variant
cat $srcdir/dat/q4 $srcdir/dat/q4 $srcdir/dat/q4 $srcdir/dat/q4 | ./entropy
# Large buffer testing
gzcat $srcdir/dat/entropy4.gz | (ulimit -s 64; ./entropy)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While ulimit -s 64 works for normal build, it's not enough for ones using -fsanitize=address,undefined as the test still fails even after merging #159. Bumping the limit up to 128 seems to do the trick, with both normal and ASAN builds failing without #159 and passing with it.

@daviesrob daviesrob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small change is needed to the ulimit to prevent failures on ASAN builds.

Neon is -c 0x0808, but we only tested from 0000 to 0404.
(On x86 this doesn't matter - it'll just be a scalar test, just as the
AVX ones on NEON are too.)

Signed-off-by: James Bonfield <jkb@sanger.ac.uk>
@jkbonfield

Copy link
Copy Markdown
Collaborator Author

A small change is needed to the ulimit to prevent failures on ASAN builds.

Adjusted it to -s 128.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants